Fix uv PEX builder for VCS and direct URL requirements#23218
Fix uv PEX builder for VCS and direct URL requirements#23218seungwoo-ji-03 wants to merge 3 commits intopantsbuild:mainfrom
Conversation
| def _parse_direct_ref_names(top_level_requirements: tuple[str, ...]) -> frozenset[str]: | ||
| """Extract canonicalized names from direct references in lockfile requirements. | ||
|
|
||
| Assumes PEX-serialized requirement strings normalize to ``name @ url``. |
There was a problem hiding this comment.
Can you back this assumption up? Even if true today, Pex doesn't guarantee it. And it seems unnecessary to assume this when each requirement string is easy to parse correctly using packaging.requirements
There was a problem hiding this comment.
And we already depend on packaging, so this wouldn't introduce a new requirement
|
I didn't pay attention to the initial PR, but this one alerted me to the fact you're parsing a Pex lock file, which I won't support. Presumably you know that and are willing to suffer breakage. Did you also know about I retro-noted this here: https://github.com/pantsbuild/pants/pull/23197/changes#r3042523328 The larger lesson is I hustled to give Pants all the tools it needed to start using uv from really any angle whatsoever. If you find yourself reaching for parsing or patching or doing much more work than calling tools, you're probably off base at this point. I may have missed something over the last 2 years setting up for this, but I don't think so. |
|
Thanks @jsirois for pointing out pex3 lock export — I wasn't aware of it when writing the initial implementation. After testing it across various scenarios (VCS refs, direct URLs, mixed PyPI+VCS, extras, etc.), it handles everything correctly and eliminates the need to parse the lockfile JSON directly. The ~450ms overhead compared to direct JSON parsing is cacheable via Pants' Process caching and only runs once per resolve, so the trade-off is well worth it. I'll open a new PR with these changes. (cc. @benjyw) |
When the uv PEX builder extracts pinned requirements from a PEX-native
lockfile, it previously formatted every package as
name==version.That breaks direct references (e.g.
git+https://..., direct HTTPS URLs,and
file://URLs), because uv then tries to resolve them via indexes.Use the lockfile's top-level
requirementsarray—where PEX preservesoriginal PEP 508 requirement strings—to detect direct references
(
name @ url). For those packages, emitname @ <artifact url>(usingthe first artifact URL from
locked_requirements) so uv fetches from theoriginal source.
Also harden lockfile parsing against malformed inputs:
artifacts: nullandartifacts: [null]safely.TypeErrorandAttributeErrorin the parsing path so malformedlockfile shapes gracefully fall back to transitive uv resolution.
Follow-up to #23197 (reported by @benjyw).
LLM Disclosure
Code was written by the author. Claude was used to help identify edge cases.